Support DPPL 0.37 #2550

mhauru · 2025-05-15T15:18:03Z

Currently in a very unfinished state.

yebai · 2025-05-18T21:30:06Z

Lots of interface code changes here are unnecessary and are planned to be removed in #2413

Perhaps we should address #2413 first.

mhauru · 2025-05-20T10:49:44Z

That is tempting, to not waste time fixing code that's on its way out. I worry though that removing the samplers will take a while still, and in the meanwhile all the accumulator stuff, and other DPPL changes that build on it, would be held back from Turing.jl. For instance, introducing ValuesAsInModelAccumulator would cut our inference time in #2542 by half.

penelopeysm · 2025-07-17T13:33:42Z

IMO, it goes both ways. Reducing sampler complexity would make this PR easier. On the other hand, merging this PR would also make it easier to remove the duplicate samplers. (To be precise, DPPL 0.37 would make it easier.)

I think DPPL 0.37 has taken a long time and we should prioritise this, rather than trying to squeeze in the changes to samplers.

I'm going to fix the merge conflicts and add a [sources] bit to Project.toml to point to the unreleased DPPL branch, so that CI can at least run on 1.11. (See https://pkgdocs.julialang.org/v1/toml-files/#The-%5Bsources%5D-section)

Note that 1.10 CI will always fail as [sources] isn't understood on 1.10.

penelopeysm · 2025-07-18T17:53:00Z

test/mcmc/hmc.jl

+    # TODO(mhauru) Do we give up being able to sample from only prior/likelihood like this,
+    # or do we implement some way to pass `whichlogprob=:LogPrior` through `sample`?


sample(::LogDensityFunction) would solve that, since we could set getlogprior in the LDF, but I guess that'll be the next release.

penelopeysm · 2025-07-19T01:04:14Z

src/mcmc/hmc.jl

-        # TODO(penelopeysm): Can we just use leafcontext(model.context)? Do we
-        # need to pass in the sampler? (In fact LogDensityFunction defaults to
-        # using leafcontext(model.context) so could we just remove the argument
-        # entirely?)
-        DynamicPPL.SamplingContext(rng, spl, DynamicPPL.leafcontext(model.context));
-        adtype=spl.alg.adtype,


As established in (e.g.) TuringLang/DynamicPPL.jl#955 (comment) SamplingContext for Hamiltonians was never overloaded so it is equivalent to just use DefaultContext in the LDF.

src/mcmc/hmc.jl

penelopeysm · 2025-07-19T01:31:57Z

src/optimisation/Optimisation.jl

+Exactly like DynamicPPL.LogPriorAccumulator, but does not include the log determinant of the
+Jacobian of any variable transformations.
+
+Used for MAP optimisation.


I am actually quite confused as to why this is needed (and OptimizationContext before it) because if the VarInfo is linked, calling getlogp on it won't include the logjac. (This is the source of the problem in #2617)

This would only be needed if we wanted to unconditionally ignore logjac even if the VarInfo is unlinked.

Oh, I think there's a terminology mismatch here. I must be thinking of logjac of the invlink transform (and that's also what #2617 refers to), whereas this is talking about logjac of the link transform.

i.e. you want to plug in values in linked space and get the invlinked logp as the optimisation target, ignoring the logjac required to transform values to linked space.

whereas the way I've been seeing it is that we can sample in linked space but we need to make sure to include the logjac of the invlink transform so that we get the invlinked logp.

penelopeysm · 2025-07-19T14:42:45Z

Because Turing re-exports some things that were changed in DPPL 0.37 (for example, LogDensityFunction), this has to go into breaking. (It probably philosophically should anyway, since I think this might be the biggest makeover that DPPL has had in a while.)

github-actions · 2025-07-19T23:33:15Z

Turing.jl documentation for PR #2550 is available at:
https://TuringLang.github.io/Turing.jl/previews/PR2550/

penelopeysm · 2025-07-20T01:10:36Z

This is getting too unwieldy, unfortunately.

codecov · 2025-07-20T16:05:40Z

Codecov Report

Attention: Patch coverage is 31.61290% with 106 lines in your changes missing coverage. Please review.

Project coverage is 24.79%. Comparing base (465642e) to head (11a2a31).

Files with missing lines	Patch %	Lines
src/mcmc/particle_mcmc.jl	0.00%	21 Missing ⚠️
src/optimisation/Optimisation.jl	69.64%	17 Missing ⚠️
src/mcmc/mh.jl	0.00%	12 Missing ⚠️
ext/TuringOptimExt.jl	0.00%	9 Missing ⚠️
src/mcmc/ess.jl	0.00%	8 Missing ⚠️
src/mcmc/is.jl	0.00%	8 Missing ⚠️
src/mcmc/gibbs.jl	36.36%	7 Missing ⚠️
src/mcmc/prior.jl	0.00%	5 Missing ⚠️
src/mcmc/sghmc.jl	0.00%	5 Missing ⚠️
ext/TuringDynamicHMCExt.jl	0.00%	3 Missing ⚠️
... and 5 more

❗ There is a different number of reports uploaded between BASE (465642e) and HEAD (11a2a31). Click for more details.

HEAD has 20 uploads less than BASE

Flag BASE (465642e) HEAD (11a2a31)

25 5

Additional details and impacted files

@@              Coverage Diff              @@
##           breaking    #2550       +/-   ##
=============================================
- Coverage     84.78%   24.79%   -60.00%     
=============================================
  Files            22       22               
  Lines          1466     1460        -6     
=============================================
- Hits           1243      362      -881     
- Misses          223     1098      +875

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

mhauru

I went through the code and highlighted with comments any bits that seemed like they weren't done.

With @penelopeysm we agreed that this PR is getting unwieldy. All the obvious interface changes should now be done. The things that don't work are getting logp from samplers, Gibbs, and particle MCMC. We should make individual PRs to fix those, merge them into this one, and then check the comments I left to make sure everything is done. Except for the comments raised, I consider the code in here currently to be ready to merge once tests pass.

@penelopeysm feel free to likewise flag any parts of the code that we must not forget to attend to before merging this. Hopefully we can then not do another review of what's here now.

mhauru · 2025-07-21T11:03:56Z

Project.toml

+[sources]
+DynamicPPL = {url = "https://github.com/TuringLang/DynamicPPL.jl", rev = "breaking"}


Must not forget to remove this.

mhauru · 2025-07-21T11:04:51Z

ext/TuringDynamicHMCExt.jl

+    # TODO(DPPL0.37/penelopeysm): This is obviously incorrect. Fix this.
+    vi = DynamicPPL.setloglikelihood!!(vi, Q.ℓq)
+    vi = DynamicPPL.setlogprior!!(vi, 0.0)


Must not forget to fix this.

mhauru · 2025-07-21T11:21:03Z

src/mcmc/external_sampler.jl

+        # TODO(DPPL0.37/penelopeysm) This is obviously wrong. Note that we
+        # have the same problem here as in HMC in that the sampler doesn't
+        # tell us about how logp is broken down into prior and likelihood.
+        # We should probably just re-evaluate unconditionally. A bit
+        # unfortunate.
+        DynamicPPL.setlogprior!!(new_varinfo, 0.0)
+        DynamicPPL.setloglikelihood!!(new_varinfo, new_logp)


Must not forget to fix this.

mhauru · 2025-07-21T11:22:51Z

src/mcmc/gibbs.jl

+        # TODO(mhauru) Fix accumulation here. In this branch anything that gets
+        # accumulated just gets discarded with `_`.
+        value, _ = DynamicPPL.tilde_assume(


Must not forget to fix this.

mhauru · 2025-07-21T11:25:47Z

src/mcmc/hmc.jl

+        # Re-evaluate to calculate log probability density.
+        # TODO(penelopeysm): This seems a little bit wasteful. Unfortunately,
+        # even though `t.stat.log_density` contains some kind of logp, this
+        # doesn't track prior and likelihood separately but rather a single
+        # log-joint (and in linked space), so which we have no way to decompose
+        # this back into prior and likelihood. I don't immediately see how to
+        # solve this without re-evaluating the model.
+        _, vi = DynamicPPL.evaluate!!(model, vi)


Must not forget to attend to this and make a call on whether to pay the cost of re-evaluating or live without a logp.

mhauru · 2025-07-21T11:28:26Z

src/mcmc/mh.jl

+    # TODO(DPPL0.37/penelopeysm): This is obviously incorrect. We need to
+    # re-evaluate the model.
    set_namedtuple!(vi, trans.params)
-    return setlogp!!(vi, trans.lp)
+    vi = DynamicPPL.setloglikelihood!!(vi, trans.lp)
+    return DynamicPPL.setlogprior!!(vi, 0.0)


Must not forget to fix this.

mhauru · 2025-07-21T11:29:09Z

src/mcmc/mh.jl

+    # TODO(DPPL0.37/penelopeysm): This is obviously incorrect. We need to
+    # re-evaluate the model.
+    vi = DynamicPPL.unflatten(vi, trans.params)
+    vi = DynamicPPL.setloglikelihood!!(vi, trans.lp)
+    return DynamicPPL.setlogprior!!(vi, 0.0)


Must not forget to fix this.

mhauru · 2025-07-21T11:30:52Z

src/mcmc/particle_mcmc.jl

+# TODO(DPPL0.37/penelopeysm) The whole tilde pipeline for particle MCMC needs to be
+# thoroughly fixed.


Must not forget to fix this.

mhauru · 2025-07-21T11:36:52Z

test/Project.toml

+[sources]
+DynamicPPL = {url = "https://github.com/TuringLang/DynamicPPL.jl", rev = "breaking"}


Must not forget to remove this.

penelopeysm · 2025-07-21T14:35:28Z

Seems to me that there are several stages:

I'll go off and get started on the logjac acc, shouldn't be too hard since it's all the same code as before.

github-actions bot assigned mhauru May 15, 2025

mhauru mentioned this pull request May 30, 2025

VariableOrderAccumulator TuringLang/DynamicPPL.jl#940

Merged

penelopeysm reviewed Jul 18, 2025

View reviewed changes

penelopeysm reviewed Jul 19, 2025

View reviewed changes

src/mcmc/hmc.jl Outdated Show resolved Hide resolved

penelopeysm reviewed Jul 19, 2025

View reviewed changes

mhauru and others added 8 commits July 19, 2025 15:46

First efforts towards DPPL 0.37 compat, WIP

cea1f7d

More DPPL 0.37 compat work, WIP

5d860d9

Add [sources] for [email protected]

c7c4638

Remove context argument from LogDensityFunction

f16a5cf

Fix MH

98d5e7a

Remove spurious logging

73e127b

Remove residual OptimizationContext

ce0c782

Delete files that were removed in previous releases

4d03c07

penelopeysm force-pushed the mhauru/dppl-0.37 branch from 4ee5649 to 4d03c07 Compare July 19, 2025 14:46

penelopeysm changed the base branch from main to breaking July 19, 2025 14:46

penelopeysm added 4 commits July 19, 2025 23:45

Fix typo

06fec2d

Simplify ESS

0af8725

Fix LDF

3d44c12

Fix Prior(), fix a couple more imports

a1837b5

penelopeysm added 5 commits July 20, 2025 00:46

fixes

17efb8c

actually fix prior

d62ad82

Remove extra return value from tilde_assume

aac93f1

fix ldf

e903d1c

actually fix prior

fd5a815

penelopeysm added 12 commits July 20, 2025 01:04

fix HMC log-density

10a130a

fix ldf

c630723

fix make_evaluate_...

9cbb2e9

more fixes for evaluate!!

335cd2a

fix hmc

c912fb9

fix run_ad

195f819

even more fixes (oh goodness when will this end)

cd52e9f

more fixes

9360f18

fix

64ebd92

more fix fix fix

283d4dd

fix return values of tilde pipeline

b346198

even more fixes

9012774

penelopeysm added 5 commits July 20, 2025 12:42

Fix missing import

e600589

More MH fixes

3d5072f

Fix conversion

37466cc

don't think it really needs those type params

1b73e5a

implement copy for LogPriorWithoutJacAcc

66a8544

penelopeysm added 2 commits July 20, 2025 17:25

Even more fixes

98e70c2

More fixes; I think the remaining failures are pMCMC related

d2c1c92

mhauru mentioned this pull request Jul 21, 2025

AdvancedPS v0.7 (and thus Libtask v0.9) support #2585

Merged

mhauru commented Jul 21, 2025

View reviewed changes

penelopeysm added 2 commits July 21, 2025 15:19

Merge branch 'breaking' into mhauru/dppl-0.37

a21f24d

Fix merge

11a2a31

		# TODO(mhauru) Do we give up being able to sample from only prior/likelihood like this,
		# or do we implement some way to pass `whichlogprob=:LogPrior` through `sample`?

		[sources]
		DynamicPPL = {url = "https://github.com/TuringLang/DynamicPPL.jl", rev = "breaking"}

		# TODO(DPPL0.37/penelopeysm) The whole tilde pipeline for particle MCMC needs to be
		# thoroughly fixed.

Support DPPL 0.37 #2550

Are you sure you want to change the base?

Support DPPL 0.37 #2550

Uh oh!

Conversation

mhauru commented May 15, 2025

Uh oh!

yebai commented May 18, 2025

Uh oh!

mhauru commented May 20, 2025

Uh oh!

penelopeysm commented Jul 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

penelopeysm Jul 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

penelopeysm Jul 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

penelopeysm commented Jul 19, 2025

Uh oh!

github-actions bot commented Jul 19, 2025

Uh oh!

penelopeysm commented Jul 20, 2025

Uh oh!

codecov bot commented Jul 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mhauru left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

penelopeysm commented Jul 21, 2025

Uh oh!

Uh oh!

penelopeysm commented Jul 17, 2025 •

edited

Loading

penelopeysm Jul 19, 2025 •

edited

Loading

penelopeysm Jul 19, 2025 •

edited

Loading

codecov bot commented Jul 20, 2025 •

edited

Loading